[libc++] Make `<map>` `std::map` constexpr as part of P3372R3 #134330

vinay-deshmukh · 2025-04-04T00:41:50Z

Depends on:

Summary:

Apply _LIBCPP_CONSTEXPR_SINCE_CXX26 to map, __tree, node-handle, __libcpp_erase_if, __lazy_synth_three_way_comparator, __lazy_compare_result, libcxx/include/__utility/try_key_extraction.h
Skip test erase_if.pass.cpp for GCC during constant evaluation. AFAICT this appears to be a g++ bug, as the test passes with clang
Disable test for node-handle::key() during constant evaluation (CWG2514). It is annotated with a // FIXME
map.modifiers/try.emplace.pass.cpp : Start using the previously unused mv3. (Should this be a separate patch?)
Has a TODO for multimap and ohters
a. libcxx/test/std/containers/associative/map/map.ops/contains.pass.cpp
b. libcxx/test/std/containers/associative/map/map.ops/contains_transparent.pass.cpp
a. libcxx/test/std/containers/container.node/node_handle.pass.cpp
~~Fix typo in libcxx/include/__memory/pointer_traits.h~~ -> [NFC][libc++] Fix typo in libcxx/include/__memory/pointer_traits.h #157304
pair<const MoveOnly, ...>
a. move_assign.pass.cpp
b. move_alloc.pass.cpp
c. Fails to compile if static_assert(test()); is called in the test file
d. Has a // FIXME with commented code
~~Change __tree_node to construct it's __node_value_type during construction to avoid the: note: member call on object outside its lifetime is not allowed in a constant expression issue.~~ This became -> [libc++] Remove UB from std::__tree_node construction #153908

github-actions · 2025-04-04T00:43:27Z

⚠️ We detected that you are using a GitHub private e-mail address to contribute to the repo.
Please turn off Keep my email addresses private setting in your account.
See LLVM Discourse for more information.

github-actions · 2025-04-04T00:44:15Z

✅ With the latest revision this PR passed the C/C++ code formatter.

vinay-deshmukh · 2025-09-12T10:01:28Z

@frederick-vs-ja can you reopen this when you get a chance? I don't see a reopen button

I think it keeps getting closed because I linked a PR review comment in the previous PR, and unfortunately GitHub thinks that implies the PR should also be closed. (And is there a way to explicitly disable that feature for one PR?)

…P3372-constexpr-map

vinay-deshmukh · 2025-09-22T10:04:00Z

@frederick-vs-ja @philnik777

Can you take a look at this pull request when you have a chance? Thank you

vinay-deshmukh · 2025-09-29T22:46:40Z

@frederick-vs-ja @philnik777

bump
Can you take a look at this pull request when you have a chance? Thank you

philnik777 · 2025-09-30T06:11:05Z

libcxx/docs/ReleaseNotes/22.rst

  by up to 2.5x
 - The performance of ``map::insert_or_assign`` has been improved by up to 2x
 - ``ofstream::write`` has been optimized to pass through large strings to system calls directly instead of copying them
+- P3372R3: ``constexpr map``(`Github <https://github.com/llvm/llvm-project/issues/128660>`__) (The paper is partially implemented. ``constexpr map`` is implemented in this release)


Please put this in the Implemented Papers section (and update the URL).

philnik777 · 2025-09-30T06:12:54Z

libcxx/include/__node_handle

+  /*
+  As this method this->key() returns a non-const reference
+  to the `key_type` which is const inside `node_type`.
+  This means that we can't make this member method constexpr until CWG2514 is resolved.
+  This disallows making the member function constexpr.
+
+  More here:
+  * https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2025/p3372r3.html#node-handle-key
+  * https://cplusplus.github.io/CWG/issues/2514.html
+  */


We really don't need a long explanation for why the standard decided to do certain things. A not that it's not constexpr according to the standard seems good enough to me.

Also, we don't use C-style comments usually.

Fixed in cdac01f

philnik777 · 2025-09-30T06:14:10Z

libcxx/include/__tree

 //           while preserving in-order order.
 template <class _NodePtr>
-_LIBCPP_HIDE_FROM_ABI void __tree_right_rotate(_NodePtr __x) _NOEXCEPT {
+_LIBCPP_CONSTEXPR_SINCE_CXX26 _LIBCPP_HIDE_FROM_ABI void __tree_right_rotate(_NodePtr __x) _NOEXCEPT {


Please stay consistent with the ordering.

Fixed in d1dfb7f

philnik777 · 2025-09-30T06:14:30Z

libcxx/include/__tree

  pointer __left_;

-  _LIBCPP_HIDE_FROM_ABI __tree_end_node() _NOEXCEPT : __left_() {}
+  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX26 __tree_end_node() _NOEXCEPT : __left_(nullptr) {}


Suggested change

_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX26 __tree_end_node() _NOEXCEPT : __left_(nullptr) {}

_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX26 __tree_end_node() _NOEXCEPT : __left_() {}

Unrelated change?

I think this may have been an attempt to try to fix the G++-15 error / arm CI error. Can be reverted

~~Fixed in 0e10680~~

This is actually a needed change.

if I revert it to __left_(), I get the following test failure:

# | /llvm/libcxx/test/std/containers/associative/map/map.access/at.pass.cpp:185:17: error: static assertion expression is not an integral constant expression # | 185 | static_assert(test()); # | | ^~~~~~ # | /llvm/libcxx/test/support/min_allocator.h:224:64: note: read of uninitialized object is not allowed in a constant expression # | 224 | TEST_CONSTEXPR_CXX14 explicit operator bool() const { return ptr_ != nullptr; } # | | ^~~~ # | /llvm/build/myclang22/libcxx/test-suite-install/include/c++/v1/__memory/pointer_traits.h:263:10: note: in call to '__p.operator bool()' # | 263 | return __p ? __ptr_traits::pointer_to(*static_cast<__element_type*>(std::addressof(*__p))) # | | ^~~ # | /llvm/build/myclang22/libcxx/test-suite-install/include/c++/v1/__tree:915:12: note: in call to '__static_fancy_pointer_cast<min_pointer<std::__tree_node<std::__value_type<int, double>, min_pointer<void>>>, min_pointer<std::__tree_node_base<min_pointer<void>>>>(m.__tree_.struct (anonymous struct at /llvm/build/myclang22/libcxx/test-suite-install/include/c++/v1/__tree:883:3).__end_node_.__left_)' # | 915 | return std::__static_fancy_pointer_cast<__node_pointer>(__end_node()->__left_); # | | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ # | /llvm/build/myclang22/libcxx/test-suite-install/include/c++/v1/__tree:1130:9: note: in call to 'this->__root()' # | 1130 | if (__root() == nullptr) { # | | ^~~~~~~~ # | /llvm/build/myclang22/libcxx/test-suite-install/include/c++/v1/map:1150:5: note: in call to 'this->__tree_.__insert_range_unique<std::pair<const int, double> *, std::pair<const int, double> *>(&ar[0], &ar[7])' # | 1150 | __tree_.__insert_range_unique(__first, __last); # | | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ # | /llvm/build/myclang22/libcxx/test-suite-install/include/c++/v1/map:969:5: note: in call to 'this->insert<std::pair<const int, double> *>(&ar[0], &ar[7])' # | 969 | insert(__f, __l); # | | ^~~~~~~~~~~~~~~~ # | /llvm/libcxx/test/std/containers/associative/map/map.access/at.pass.cpp:113:61: note: in call to 'map<std::pair<const int, double> *>(&ar[0], &ar[7], key_compare())' # | 113 | std::map<int, double, std::less<int>, min_allocator<V>> m(ar, ar + sizeof(ar) / sizeof(ar[0])); # | | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ # | /llvm/libcxx/test/std/containers/associative/map/map.access/at.pass.cpp:185:17: note: in call to 'test()' # | 185 | static_assert(test());

philnik777 · 2025-09-30T06:15:00Z

libcxx/include/__tree

+  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX26 __tree_node_base()  = default;
+  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX26 ~__tree_node_base() = default;


Again, unrelated change?

Fixed in b556c8426391

philnik777 · 2025-09-30T06:15:44Z

libcxx/include/__tree

 public:
-  _LIBCPP_HIDE_FROM_ABI __node_value_type& __get_value() { return *reinterpret_cast<__node_value_type*>(__buffer_); }
+  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX26 __node_value_type& __get_value() {
+    return *reinterpret_cast<__node_value_type*>(__buffer_);


Either this reinterpret_cast is not needed or the constexpr annotation.

This __get_value() should be left unchanged. It's only used in C++03 mode.

Fixed in 8ecbbb5ac277

philnik777 · 2025-09-30T06:16:28Z

libcxx/include/__tree

+// TODO: workaround for arm CI which uses clang-19.1.4
+/*
+note: non-literal type 'std::__tree_node<std::__value_type<int, int>, void *>' cannot be used in a constant expression
+*/


Can we just wait for the bots to be upgraded? It's currently in-progress.

Good to know! Is there a github issue/pull request tracking that change?

philnik777 · 2025-09-30T06:16:51Z

libcxx/include/__tree

-  _LIBCPP_HIDE_FROM_ABI __tree_node_destructor(const __tree_node_destructor&) = default;
-  __tree_node_destructor& operator=(const __tree_node_destructor&)            = delete;
+  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX26 __tree_node_destructor(const __tree_node_destructor&) = default;
+  _LIBCPP_CONSTEXPR_SINCE_CXX26 __tree_node_destructor& operator=(const __tree_node_destructor&)            = delete;


= deleted members don't need annotations.

philnik777 · 2025-09-30T06:21:07Z

libcxx/include/__tree

+      __node_allocator& __na = __node_alloc();
+      __node_traits::destroy(__na, std::addressof(__lhs));
+
+      using __node_value_type = __get_node_value_type_t<value_type>;
+      __node_value_type __tmp(__rhs.first, __rhs.second);
+
+      __node_traits::construct(__na, std::addressof(__lhs), std::move(__tmp));


This is a significant performance regression as-is. We should only do this during constant evaluation.

c28aac0 and c448834 solve this, but increase the "complexity" of the code. I've added comments on why that's needed for now, but can be removed as needed.

My understanding so far:

if we use if constexpr, this means for all types that satisfy the copy-constructible trait, the code will always prefer copying. Types that don't satisfy this trait, will fail to compile during constant evaluation

if (std::is_constant_evaluated() allows us to "diverge" the runtime/compile time code paths, which means removing the constexpr otherwise the compiler yells saying if constexpr(std::is_constant_evaluated() && other) is always true.

However, that change means any code within if must compile (instantiate valid template code), which is not true when dealing with non-copy-constructible types.

Hence, to delay the template instantiation, and allow compiling (hence, defer to the original runtime code path with UB), I defined a new overload that "overloads" on the true_type/false_type argument, similar to:

llvm-project/libcxx/include/__tree

Lines 1223 to 1232 in 0aa7da0

_LIBCPP_HIDE_FROM_ABI void __copy_assign_alloc(const __tree& __t) {

__copy_assign_alloc(__t, integral_constant<bool, __node_traits::propagate_on_container_copy_assignment::value>());

}

_LIBCPP_HIDE_FROM_ABI void __copy_assign_alloc(const __tree& __t, true_type) {

if (__node_alloc() != __t.__node_alloc())

clear();

__node_alloc() = __t.__node_alloc();

}

_LIBCPP_HIDE_FROM_ABI void __copy_assign_alloc(const __tree&, false_type) {}

frederick-vs-ja · 2025-09-30T06:38:23Z

libcxx/include/__tree

  __insert_unique_from_orphaned_node(const_iterator __p, __get_node_value_type_t<_Tp>&& __value) {
-    __emplace_hint_unique(__p, const_cast<key_type&&>(__value.first), std::move(__value.second));
+#if _LIBCPP_STD_VER >= 26
+    if constexpr (integral_constant<bool, is_copy_constructible<decltype(__value.first)>::value >()) {


Can't we just use if constexpr(is_copy_constructible_v<...>? Also, let's only use this branch in constant evaluation.

Simplified here: 2661937

for the "only in constant evaluation part, I've had to change the code a bit, which has been done in c28aac0 and c448834. More here: #134330 (comment)

frederick-vs-ja · 2025-09-30T06:39:15Z

libcxx/include/__tree

-    __lhs.second                         = std::forward<_From>(__rhs).second;
+#if _LIBCPP_STD_VER >= 26
+
+    if constexpr (integral_constant<bool, is_copy_constructible<decltype(__rhs.first)>::value >()) {


Ditto, if constexpr (is_copy_constructible_v<decltype(__rhs.first)>) seems OK.

Simplification done here: 2661937

frederick-vs-ja · 2025-09-30T06:42:50Z

libcxx/test/std/containers/associative/map/map.access/at.pass.cpp

-      TEST_IGNORE_NODISCARD m.at(6);
-      assert(false);
-    } catch (std::out_of_range&) {
+#  if TEST_STD_VER >= 26


I think we can use if TEST_STD_AT_LEAST_26_OR_RUNTIME_EVALUATED { /* ... */ }. Ditto for all throwing cases.

I'm not sure if that macro can be used here, as the condition here appears to be TEST_STD_AT_LEAST_26_AND_RUNTIME_EVALUATED

And that macro always returns true if TEST_STD_VER >= 26

vinay-deshmukh force-pushed the vinay-issue-128660-P3372-constexpr-map branch 2 times, most recently from 46d0bb9 to 9ea718f Compare April 7, 2025 22:25

vinay-deshmukh added 26 commits April 8, 2025 21:43

failing at copy_assign.pass.cpp only in constexpr

1e86d9c

pass deduct.pass.cpp

85e1cfc

fix map.cons/from_range.pass.cpp

6ebe12c

fail at map.cons/move_alloc.pass.cpp

ff72cf9

fail at map.cons/move_assign.pass.cpp

76c02b7

fix map.cons/move_noexcept.pass.cpp

2a7d238

pass map.erasure/erase_if.pass.cpp

f188a7b

error: call to deleted constructor at map.modifiers/emplace.pass.cpp

6a08bff

pass map.modifiers/erase_iter.pass.cpp

2b47aba

pass map.modifiers/extract_iterator.pass.cpp

e045092

TODO: node-handle tests

d61d023

pass map.modifiers/extract_key.pass.cpp

5b564ff

pass map.modifiers/insert_and_emplace_allocator_requirements.pass.cpp

d17a5f4

pass map.modifiers/insert_cv.pass.cpp

9e3fff0

pass map.modifiers/insert_iter_cv.pass.cpp

662d172

pass map.modifiers/insert_iter_rv.pass.cpp

3c39ba7

pass map.modifiers/insert_node_type.pass

a462c8e

pass map.modifiers/insert_node_type_hint.pass.cpp

d57d651

pass map.modifiers/insert_or_assign.pass.cpp

8c94f4d

pass map.modifiers/insert_rv.pass.cpp

dc5fbee

pass map.modifiers/try.emplace.pass.cpp, fix typos

674854d

pass map.nonmember/compare.three_way.pass.cpp

a00a45a

pass map.nonmember/op_compare.pass.cpp

3728788

pass map.observers/key_comp.pass.cpp

490645c

pass map.observers/value_comp.pass.cpp

f136d58

pass map.ops/contains.pass.cpp

bef3a7f

vinay-deshmukh added 3 commits September 11, 2025 19:42

obsolete comment

2ef6175

Add FIXME

8c78c45

Apply cf

7616770

vinay-deshmukh requested review from philnik777 and frederick-vs-ja September 11, 2025 23:54

simpal01 closed this in simpal01/arm-toolchain@daa88b3 Sep 12, 2025

Zingam reopened this Sep 13, 2025

vinay-deshmukh added 6 commits September 13, 2025 08:29

Merge remote-tracking branch 'upstream/main' into vinay-issue-128660-…

a6c32e1

…P3372-constexpr-map

Merge remote-tracking branch 'upstream/main' into vinay-issue-128660-…

a6eb443

…P3372-constexpr-map

Merge remote-tracking branch 'upstream/main' into vinay-issue-128660-…

dcc1533

…P3372-constexpr-map

cf

28ed94b

add constexpr

3bbf31d

cf

a209fb5

philnik777 reviewed Sep 30, 2025

View reviewed changes

frederick-vs-ja reviewed Sep 30, 2025

View reviewed changes

vinay-deshmukh added 12 commits September 30, 2025 18:58

Move ReleaseNotes to Implemented papers

b083bea

Fix order of annotations

d1dfb7f

node-handle doc

cdac01f

unrelated defaulting

0e10680

undo previous

c7307a8

unnecessary destructor for __tree_node_base

b556c84

Remove constexpr from cpp03 code

8ecbbb5

Unnecessary annotation on deleted function

2f2bec2

simplify type_trait

2661937

Allow runtime decision for __insert_unique_from_orphaned_node

c28aac0

Allow runtime decision for __assign_value

c448834

local cf

f732308

	_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX26 __tree_end_node() _NOEXCEPT : __left_(nullptr) {}
	_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX26 __tree_end_node() _NOEXCEPT : __left_() {}

	_LIBCPP_HIDE_FROM_ABI void __copy_assign_alloc(const __tree& __t) {
	__copy_assign_alloc(__t, integral_constant<bool, __node_traits::propagate_on_container_copy_assignment::value>());
	}

	_LIBCPP_HIDE_FROM_ABI void __copy_assign_alloc(const __tree& __t, true_type) {
	if (__node_alloc() != __t.__node_alloc())
	clear();
	__node_alloc() = __t.__node_alloc();
	}
	_LIBCPP_HIDE_FROM_ABI void __copy_assign_alloc(const __tree&, false_type) {}

[libc++] Make <map> std::map constexpr as part of P3372R3 #134330

Are you sure you want to change the base?

[libc++] Make <map> std::map constexpr as part of P3372R3 #134330

Uh oh!

Conversation

vinay-deshmukh commented Apr 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Apr 4, 2025

Uh oh!

github-actions bot commented Apr 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vinay-deshmukh commented Sep 12, 2025

Uh oh!

vinay-deshmukh commented Sep 22, 2025

Uh oh!

vinay-deshmukh commented Sep 29, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vinay-deshmukh Sep 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vinay-deshmukh Oct 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vinay-deshmukh Oct 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

[libc++] Make `<map>` `std::map` constexpr as part of P3372R3 #134330

[libc++] Make `<map>` `std::map` constexpr as part of P3372R3 #134330

vinay-deshmukh commented Apr 4, 2025 •

edited

Loading

github-actions bot commented Apr 4, 2025 •

edited

Loading

vinay-deshmukh Sep 30, 2025 •

edited

Loading

vinay-deshmukh Oct 1, 2025 •

edited

Loading

vinay-deshmukh Oct 1, 2025 •

edited

Loading